[Themes] Refactor CLI server events for Hot Reload (PoC Hot Reload for OSE) #4908

frandiox · 2024-11-22T15:47:32Z

Goals: support hot reload for Online Store Editor and Theme Preview.

Full context here.

The changes in this repo are:

Move the hot reload decisions from the server to the client. For example, instead of sending an event for "CSS update" from the server to the client, now the server sill simply say "here, this file has been changed". The client will check the extension of the file and decide to perform a CSS update, or something else.
The server now emits 2 events per file: once when the file is updated locally, and another one when it's uploaded to the cloud. This means that the client can choose to refresh earlier or later, depending of what it needs. For example, when running on localhost, hot reload will happen as soon as a file is updated locally because we can access the content. However, in OSE, we will wait for the "cloud sync" event before triggering a refresh.
Implement handling for events in the client for multiple situations: CLI, OSE, Theme Preview. This is done in https://github.com/shopify/theme-hot-reload/
If the hot reload frontend is not injected by SFR, the CLI will inject it. This way we can release without depending on SFR deploys.

🎩 :

shopify theme dev and check that it downloads the theme-hot-reload script in the network tab.
Hot reload should keep working the same in localhost for every file type.

github-actions · 2024-11-22T15:51:43Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	75.32% (-0.05% 🔻)	8889/11801
🟡	Branches	70.56% (-0.04% 🔻)	4322/6125
🟡	Functions	75.1% (-0.05% 🔻)	2337/3112
🟡	Lines	75.86% (-0.04% 🔻)	8401/11074

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / app-event-watcher.ts	93.83% (-1.23% 🔻)	86.49% (-2.7% 🔻)	90.48%	98.61%
🟡	`...` / build.ts	78.51% (-0.18% 🔻)	64.58%	78.38%	76.99% (-0.2% 🔻)
🟢	`...` / ConcurrentOutput.tsx	98.36% (-1.64% 🔻)	88% (-4% 🔻)	100%	98.33% (-1.67% 🔻)
🟡	`...` / fs.ts	66.67% (-2.22% 🔻)	93.75%	63.64% (-2.27% 🔻)	65.91% (-2.27% 🔻)
🟢	`...` / git.ts	88.73% (-1.51% 🔻)	87.18% (-1.71% 🔻)	87.5% (-1.39% 🔻)	90% (-1.25% 🔻)

Test suite run success

2001 tests passing in 904 suites.

Report generated by 🧪jest coverage report action from 8657282

frandiox · 2025-01-10T18:45:37Z

/snapit

github-actions · 2025-01-10T18:47:19Z

🫰✨ Thanks @frandiox! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts

karreiro

Thank you for this PR, @frandiox 🔥 Amazing work as usual!

I couldn't spot any issues with themes neither theme app extension. Given the nature of this change tho, I'd love to expand these tests even further.

When we feel ready, I believe we can snapit this branch and share a build with our partners to gather their feedback before the release. I'm sure they will love having hot reload on OSE, so we will likely have a wide audience.

Regarding these changes:

Move the hot reload decisions from the server to the client. For example, instead of sending an event for "CSS update" from the server to the client, now the server sill simply say "here, this file has been changed". The client will check the extension of the file and decide to perform a CSS update, or something else.

This is very neat. The more I look at the client.ts file, the more it feels like it should be a standalone library. I can see it being a dependency for the CLI and for the OSE/theme-preview, and I can also see third-party tooling being built on top of it.

Having that as a standalone library might also give us more room to separate the logic and version those APIs—keeping in mind that OSE won't have guarantees about the CLI version.

The server now emits 2 events per file: once when the file is updated locally, and another one when it's uploaded to the cloud

This is an excellent decision! ✨

Please, let me know your thoughts about this! Thanks again for this PR!

packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts

…ors in OSE when CLI is not active

github-actions · 2025-01-23T15:30:11Z

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

github-actions · 2025-01-24T17:40:01Z

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/themes/types.d.ts

@@ -4,18 +4,19 @@ import { AdminSession } from '../session.js';
  */
 export type Key = string;
 export type ThemeFSEventName = 'add' | 'change' | 'unlink';
+type OnSync = (onSuccess: () => void, onError?: () => void) => void;
 type ThemeFSEvent = {
     type: 'unlink';
     payload: {
         fileKey: Key;
-        onSync?: (fn: () => void) => void;
+        onSync: OnSync;
     };
 } | {
     type: 'add' | 'change';
     payload: {
         fileKey: Key;
         onContent: (fn: (content: string) => void) => void;
-        onSync: (fn: () => void) => void;
+        onSync: OnSync;
     };
 };
 export type ThemeFSEventPayload<T extends ThemeFSEventName = 'add'> = (ThemeFSEvent & {

karreiro

Thanks a lot for this PR, @frandiox! 🚀 It looks great! I believe we just need to handle those minor issues on the editor side, and then we can proceed with the merge here :)

Also, I've left a minor question about versioning events 🙇

karreiro · 2025-01-28T13:28:37Z

packages/theme/package.json

@@ -50,6 +50,7 @@
    "yaml": "2.7.0"
  },
  "devDependencies": {
+    "@shopify/theme-hot-reload": "^0.0.5",


Do you think we should version the events here somehow? (in runtime)

This way, if we need to change something, we can prevent older versions of the CLI from breaking (we will likely need to keep triggering old events if we really change the shape of them)

benjaminsehl · 2025-01-28T20:36:04Z

/snapit

github-actions · 2025-01-28T20:38:13Z

🫰✨ Thanks @benjaminsehl! Your snapshot has been published to npm.

Test the snapshot by intalling your package globally:

pnpm i -g @shopify/[email protected]

After installing, validate the version by running just shopify in your terminal
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

frandiox marked this pull request as ready for review January 16, 2025 19:42

frandiox requested a review from a team as a code owner January 16, 2025 19:42

frandiox commented Jan 16, 2025

View reviewed changes

packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts Outdated Show resolved Hide resolved

karreiro reviewed Jan 17, 2025

View reviewed changes

packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts Outdated Show resolved Hide resolved

packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts Outdated Show resolved Hide resolved

frandiox requested a review from a team as a code owner January 21, 2025 14:34

frandiox added 22 commits January 23, 2025 15:54

Adapt hot reloading for OSE

ed57291

Wait full sync before doing full-reload

4f8f407

Move CORS logic

9dbd2ae

Rely on section rendering API from the browser instead of cloud syncing

23cb3e1

Show HR port in OSE url

dbd0bb9

Introduce HR filters to fork logic for OSE without affecting CLI

4391c33

Fix type in app ext

1bd2a3e

Move hot reload decisions to frontend to increase client flexibility

66ee3ec

Basic support for OSE preview hot reload

0e1da8e

Support hot reload in prod Theme Preview

ed65e1e

Sections hot reload for OSE

27b1102

Temporary fix for hot reloading sections in OSE

8f661ed

Support hot reloading theme settings in OSE

f9da2b2

Support hot reload for language files in OSE

8cf4d5e

Skip SSE reconnection if it fails the first time to avoid console err…

5bf61d3

…ors in OSE when CLI is not active

Support hot reload from Online Code Editor via OSE

dd4a80c

Rename type and add support for create events

62f53ad

Send all events to OSE

b54a3e2

Fix sections reload in OSE

4fa7d47

Remove comment

01777c2

Disable outdated test

0c2b77d

Simplify hot reload endpoints

f606b06

frandiox added 6 commits January 23, 2025 15:57

Add mechanism to log to console from HMR logic

45bb31f

Use external dep for hot reloading client

3319aaa

Use local theme-hot-reload script when indicated via env vars

15056ca

Rename isAppExtension to isThemeExtension

666ef82

Restore links after bad merge

761d551

Fix types after rebase

0579cf9

frandiox force-pushed the fd-hmr-ose-v2 branch from d0761c9 to 0579cf9 Compare January 23, 2025 15:29

frandiox changed the title ~~[Themes] PoC Hot Reload for OSE~~ [Themes] Refactor CLI server events for Hot Reload (PoC Hot Reload for OSE) Jan 23, 2025

frandiox added 4 commits January 24, 2025 14:40

Get sectionKey from frontend

37bc181

Update server tests

e15251d

Bump theme-hot-reload

b606592

Rename log param

8657282

frandiox requested a review from karreiro January 24, 2025 19:09

karreiro reviewed Jan 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Themes] Refactor CLI server events for Hot Reload (PoC Hot Reload for OSE) #4908

[Themes] Refactor CLI server events for Hot Reload (PoC Hot Reload for OSE) #4908

frandiox commented Nov 22, 2024 •

edited

Loading

github-actions bot commented Nov 22, 2024 •

edited

Loading

frandiox commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

karreiro left a comment •

edited

Loading

github-actions bot commented Jan 23, 2025

github-actions bot commented Jan 24, 2025

karreiro left a comment

karreiro Jan 28, 2025

benjaminsehl commented Jan 28, 2025

github-actions bot commented Jan 28, 2025

[Themes] Refactor CLI server events for Hot Reload (PoC Hot Reload for OSE) #4908

Are you sure you want to change the base?

[Themes] Refactor CLI server events for Hot Reload (PoC Hot Reload for OSE) #4908

Conversation

frandiox commented Nov 22, 2024 • edited Loading

github-actions bot commented Nov 22, 2024 • edited Loading

Coverage report

Test suite run success

frandiox commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

karreiro left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 23, 2025

github-actions bot commented Jan 24, 2025

Differences in type declarations

New type declarations

Existing type declarations

karreiro left a comment

Choose a reason for hiding this comment

karreiro Jan 28, 2025

Choose a reason for hiding this comment

benjaminsehl commented Jan 28, 2025

github-actions bot commented Jan 28, 2025

frandiox commented Nov 22, 2024 •

edited

Loading

github-actions bot commented Nov 22, 2024 •

edited

Loading

karreiro left a comment •

edited

Loading